Bind to FLINT/NTL API for polynomial modular exponentiation (new version) #37441

GiacomoPope · 2024-02-23T15:51:17Z

This is a rejuvenation of the pull request by @remyoudompheng #35320.

All I have done is rebase with develop and modify a few lines to fit with the current structuring of SageMath. I have also added a few additional docstrings.

As an example of the performance gain:

Flint polynomials over prime fields

Old timings:

About 5x faster for flint using nmod_poly

sage: R.<x> = GF(5)[]
sage: %time pow(x+1, 5**100, x^5 + 4*x + 3)
CPU times: user 551 µs, sys: 1e+03 ns, total: 552 µs
Wall time: 555 µs
x + 1

New timings:

sage: R.<x> = GF(5)[]
sage:  %time pow(x+1, 5**100, x^5 + 4*x + 3)
CPU times: user 175 µs, sys: 1e+03 ns, total: 176 µs
Wall time: 179 µs
x + 1

NTL polynomials over prime fields

About 1000x faster for NTL $\textrm{GF}(p)$
Old timings:

sage: set_random_seed(0)
....: p = 2**255 - 19
....: R.<x> = F[]
....: f = R.random_element(degree=12)
....: g = R.random_element(degree=12)
....: %timeit pow(f, 2**33, g)
92.3 ms ± 947 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
sage: %timeit pow(f, 2**64, g)
1.29 s ± 35.5 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

New timings:

sage: set_random_seed(0)
....: p = 2**255 - 19
....: R.<x> = F[]
....: f = R.random_element(degree=12)
....: g = R.random_element(degree=12)
....: %timeit pow(f, 2**33, g)
651 µs ± 28.9 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
sage:  %timeit pow(f, 2**64, g)
1.17 ms ± 10.7 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

NTL polynomials over extension fields

About 10x faster for NTL polynomial rings over $\textrm{GF}(p^k)$
Old timings:

sage: set_random_seed(0)
....: 
....: p = 2**255 - 19
....: F.<a> = GF(p^3)
....: q = F.order()
....: R.<x> = F[]
....: 
....: f = R.random_element(degree=12)
....: g = R.random_element(degree=12)
....: 
....: %time _ = pow(f, q, g)
....: 
....: p = 29
....: F.<a> = GF(p^100)
....: q = F.order()
....: R.<x> = F[]
....: 
....: f = R.random_element(degree=12)
....: g = R.random_element(degree=12)
....: 
....: %time _ = pow(f, q, g)
CPU times: user 409 ms, sys: 1.45 ms, total: 410 ms
Wall time: 411 ms
CPU times: user 14.6 s, sys: 202 ms, total: 14.8 s
Wall time: 14.9 s

New timings:

sage: set_random_seed(0)
....: 
....: p = 2**255 - 19
....: F.<a> = GF(p^3)
....: q = F.order()
....: R.<x> = F[]
....: 
....: f = R.random_element(degree=12)
....: g = R.random_element(degree=12)
....: 
....: %time _ = pow(f, q, g)
....: 
....: p = 29
....: F.<a> = GF(p^100)
....: q = F.order()
....: R.<x> = F[]
....: 
....: f = R.random_element(degree=12)
....: g = R.random_element(degree=12)
....: 
....: %time _ = pow(f, q, g)
CPU times: user 371 ms, sys: 761 µs, total: 371 ms
Wall time: 371 ms
CPU times: user 1.54 s, sys: 10.5 ms, total: 1.55 s
Wall time: 1.55 s

Closes #35320

tscrim

Just a bunch of details basically.

src/sage/libs/ntl/ntl_ZZ_pX.pyx

src/sage/rings/polynomial/polynomial_modn_dense_ntl.pyx

src/sage/rings/polynomial/polynomial_zmod_flint.pyx

src/sage/rings/polynomial/polynomial_zz_pex.pyx

Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>

GiacomoPope · 2024-02-24T14:40:20Z

Thanks for the comments, I have pushed your suggestions and then made the necessary changes for cython compilation. ./sage -t --new is fine, but I will let the CI run randomised testing for --all.

GiacomoPope · 2024-02-24T18:54:42Z

On mobile so cannot search and fix what this failed doctest is. Will fix this ASAP

grhkm21 · 2024-02-24T20:06:24Z

It's unrelated (exists since 10.2 release), I have reported it in #37454

GiacomoPope · 2024-02-24T20:08:11Z

Thanks! Is there an easy way to trigger the CI to run again here? I could do a dumb commit but maybe there's something else I can do?

grhkm21 · 2024-02-24T20:09:03Z

Don't think so, CI/workflow is triggered on push commit

GiacomoPope · 2024-02-24T20:53:44Z

OK. Well either @tscrim will be OK with the current status after implementing his changes or I can do something silly to retrigger the CI.

grhkm21 · 2024-02-24T20:58:08Z

Sorry, I take the comment back. I do not fully understand the CI failure yet. I'm currently tracing it in #37454 ("fake failure") and #37455 (true cause).

GiacomoPope · 2024-02-24T23:05:07Z

Doctesting 1 file.
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 16.37 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 17.2 seconds
    cpu time: 16.3 seconds
    cumulative wall time: 16.4 seconds
Features detected for doctesting: sage.modules,sage.rings.finite_rings,sage.rings.number_field,sage.rings.padics,sage.symbolic
pytest is not installed in the venv, skip checking tests that rely on it

I can't even reproduce the error locally???

tscrim

Thank you. LGTM.

GiacomoPope · 2024-02-25T21:30:54Z

Note that the test failure here has been identified in #37471 and fixed in #37443 and is unrelated to this PR

grhkm21 · 2024-02-26T04:10:25Z

Can you add "closes #35320" in the PR description? :D

GiacomoPope · 2024-02-26T12:34:25Z

@tscrim Sorry for the ping. I noticed a missing backtick in one of the docstrings so I fixed this. I don't know if officially this needs "reapproval"?

tscrim · 2024-02-26T12:48:46Z

I don't think so, but it is still good for someone to take a look over the changes. All is good.

vbraun · 2024-03-26T23:09:56Z

merge conflict

GiacomoPope · 2024-03-31T23:58:43Z

Conflict resolved.

GiacomoPope · 2024-04-08T10:25:56Z

@vbraun Can you take a look to whether this needs work still? Trying to clean up some of my PR

github-actions · 2024-04-09T14:48:14Z

Documentation preview for this PR (built with commit eb80a09; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

GiacomoPope · 2024-04-10T09:29:33Z

Doctest failure:

sage: from sage.rings.tests import test_karatsuba_multiplication ## line 429 ##
sage: test_karatsuba_multiplication(ZZ, 6, 5, verbose=True, seed=42) ## line 430 ##
test_karatsuba_multiplication: ring=Univariate Polynomial Ring in x over Integer Ring, threshold=2
  (x^6 + 4*x^5 + 4*x^4 - 3*x^3 - x^2 - x)*(2*x^4 + 3*x^3 - 20*x^2 - 2*x + 1)
  (4*x^5 + 16*x^2 + x - 41)*(x^2 + x - 1)
  (8*x^2 + 2*x + 1)*(3)
  (-4*x - 1)*(-8*x^2 - x)
  (-x^6 - x^3 - x^2 + x + 1)*(2*x^3 - x + 3)
  (-x^2 + x + 1)*(x^4 + x^3 - x^2 - x + 76)
  (4*x^3 + x^2 + 6)*(-x^2 - 5*x)
  (x + 4)*(-x + 5)
  (-2*x)*(3*x^2 - x)
  (x^6 + 21*x^5 + x^4 + 4*x^3 - x^2)*(14*x^4 + x^3 + 2*x^2 - 12*x)
sage: rings = [QQ] ## line 445 ##
sage: rings += [ZZ[I], ZZ[I, sqrt(2)]]                                          # needs sage.rings.number_field sage.symbolic ## line 446 ##
sage: rings += [GF(49, 'a')]                                                    # needs sage.rings.finite_rings ## line 447 ##
sage: rings += [MatrixSpace(GF(17), 3)]                                         # needs sage.modules ## line 448 ##
sage: for C in rings:                                                           # needs sage.modules
    test_karatsuba_multiplication(C, 10, 10) ## line 449 ##
sage: test_karatsuba_multiplication(QQbar, 3, 3, numtests=2)    # long time, needs sage.rings.number_field ## line 454 ##

Is unrelated, and to do with: #37455

GiacomoPope · 2024-04-19T10:26:42Z

Sorry for the ping, @vbraun but wanted to follow up on the needs work tag.

tscrim · 2024-04-21T09:08:35Z

I think its fine to just remove the needs work tag once the merge conflict is resolved assuming it was trivial (if it was not, just request a re-review by the reviewer(s)). Now just to wait for it to be merged.

GiacomoPope · 2024-04-21T09:19:20Z

Oh ok. I was under the impression that the PR owner should never add the positive review or remove the needs work tags.

GiacomoPope added 5 commits February 23, 2024 15:06

Add fast pow for NTL polynomials over extensions

40f4d9b

Fix linting

9331b5f

update docstring

7eca602

add fast modular exponentiation for polynomial rings over Z/NZ

2750aee

fast pow mod for flint Z/NZ

68c1f10

GiacomoPope requested a review from tscrim February 23, 2024 15:51

grhkm21 added t: enhancement s: needs review c: algebra t: performance and removed t: enhancement labels Feb 24, 2024

tscrim requested changes Feb 24, 2024

View reviewed changes

GiacomoPope and others added 2 commits February 24, 2024 14:28

Apply suggestions from code review

84e5dc6

Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>

Ensure cython compiles after reviewer suggestion

10d57ed

fix change from cpdef to cdef

713af40

grhkm21 mentioned this pull request Feb 24, 2024

Method .random_element not impl'ed forLaurentPolynomialRing #37454

Closed

2 tasks

grhkm21 mentioned this pull request Feb 24, 2024

test timeout in rings/tests.py #37455

Open

2 tasks

replace trac with issue

2e98cf8

grhkm21 mentioned this pull request Feb 25, 2024

Fix bugs and regression in performance of any_root() #37443

Merged

tscrim approved these changes Feb 25, 2024

View reviewed changes

tscrim added t: enhancement and removed s: needs review labels Feb 25, 2024

Merge branch 'develop' into faster_poly_pow_mod

8eeb537

GiacomoPope added the p: major / 3 label Feb 25, 2024

Fix docstring formatting

995a94b

Merge branch 'develop' into faster_poly_pow_mod

bddd10e

vbraun added the s: needs work label Mar 26, 2024

GiacomoPope added 2 commits March 26, 2024 23:03

Merge branch 'develop' into faster_poly_pow_mod

9f7cf3f

Merge branch 'develop' into faster_poly_pow_mod

7419c5f

GiacomoPope mentioned this pull request Apr 1, 2024

RST liner is failing due to RST304 failure #37711

Closed

2 tasks

Add checks for failing doctests

a2b5799

GiacomoPope mentioned this pull request Apr 3, 2024

Test modularised distributions causes CI failure #37734

Closed

2 tasks

Merge branch 'develop' into faster_poly_pow_mod

2fc1ed4

GiacomoPope mentioned this pull request Apr 10, 2024

Random testing has revealed a problem in test_karatsuba_multiplication #35715

Open

2 tasks

Merge branch 'develop' into faster_poly_pow_mod

eb80a09

tscrim removed the s: needs work label Apr 21, 2024

vbraun merged commit b704d41 into sagemath:develop Apr 27, 2024
12 checks passed

mkoeppe added this to the sage-10.4 milestone Apr 27, 2024

github-actions bot removed the s: positive review label Apr 27, 2024

GiacomoPope deleted the faster_poly_pow_mod branch August 21, 2024 14:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bind to FLINT/NTL API for polynomial modular exponentiation (new version) #37441

Bind to FLINT/NTL API for polynomial modular exponentiation (new version) #37441

GiacomoPope commented Feb 23, 2024 •

edited

Loading

tscrim left a comment

GiacomoPope commented Feb 24, 2024

GiacomoPope commented Feb 24, 2024

grhkm21 commented Feb 24, 2024

GiacomoPope commented Feb 24, 2024

grhkm21 commented Feb 24, 2024 •

edited

Loading

GiacomoPope commented Feb 24, 2024

grhkm21 commented Feb 24, 2024

GiacomoPope commented Feb 24, 2024

tscrim left a comment

GiacomoPope commented Feb 25, 2024 •

edited

Loading

grhkm21 commented Feb 26, 2024

GiacomoPope commented Feb 26, 2024

tscrim commented Feb 26, 2024

vbraun commented Mar 26, 2024

GiacomoPope commented Mar 31, 2024

GiacomoPope commented Apr 8, 2024

github-actions bot commented Apr 9, 2024 •

edited

Loading

GiacomoPope commented Apr 10, 2024

GiacomoPope commented Apr 19, 2024

tscrim commented Apr 21, 2024

GiacomoPope commented Apr 21, 2024

Bind to FLINT/NTL API for polynomial modular exponentiation (new version) #37441

Bind to FLINT/NTL API for polynomial modular exponentiation (new version) #37441

Conversation

GiacomoPope commented Feb 23, 2024 • edited Loading

Flint polynomials over prime fields

NTL polynomials over prime fields

NTL polynomials over extension fields

tscrim left a comment

Choose a reason for hiding this comment

GiacomoPope commented Feb 24, 2024

GiacomoPope commented Feb 24, 2024

grhkm21 commented Feb 24, 2024

GiacomoPope commented Feb 24, 2024

grhkm21 commented Feb 24, 2024 • edited Loading

GiacomoPope commented Feb 24, 2024

grhkm21 commented Feb 24, 2024

GiacomoPope commented Feb 24, 2024

tscrim left a comment

Choose a reason for hiding this comment

GiacomoPope commented Feb 25, 2024 • edited Loading

grhkm21 commented Feb 26, 2024

GiacomoPope commented Feb 26, 2024

tscrim commented Feb 26, 2024

vbraun commented Mar 26, 2024

GiacomoPope commented Mar 31, 2024

GiacomoPope commented Apr 8, 2024

github-actions bot commented Apr 9, 2024 • edited Loading

GiacomoPope commented Apr 10, 2024

GiacomoPope commented Apr 19, 2024

tscrim commented Apr 21, 2024

GiacomoPope commented Apr 21, 2024

GiacomoPope commented Feb 23, 2024 •

edited

Loading

grhkm21 commented Feb 24, 2024 •

edited

Loading

GiacomoPope commented Feb 25, 2024 •

edited

Loading

github-actions bot commented Apr 9, 2024 •

edited

Loading